Skip to content

remove mavis, change on-dock-removed to event dock.removed#1207

Merged
anandkumarpatel merged 5 commits intomasterfrom
SAN-3042-remove-docks-from-consul
Dec 7, 2015
Merged

remove mavis, change on-dock-removed to event dock.removed#1207
anandkumarpatel merged 5 commits intomasterfrom
SAN-3042-remove-docks-from-consul

Conversation

@anandkumarpatel
Copy link
Copy Markdown
Contributor

  • make dock.removed an event
  • remove mavis from api ಥ_ಥ 😢 ಢ_ಢ, you will be missed

required changes in other repos
CodeNow/mavis#37

review

@anandkumarpatel anandkumarpatel changed the title change on-dock-removed to event dock-removed remove mavis, change on-dock-removed to event dock-removed Dec 5, 2015
@podviaznikov
Copy link
Copy Markdown
Member

@anandkumarpatel where/how did you handle problem with swarm scheduling on the wrong dock?

@podviaznikov
Copy link
Copy Markdown
Member

also why did you rename worker from on-dock-removed to dock-removed?

@anandkumarpatel
Copy link
Copy Markdown
Contributor Author

I decided to add code in mavis to wait for key to be removed. It will be a cleaner solution and be what the event 'dock-removed' actually means.

CodeNow/mavis#37

@anandkumarpatel anandkumarpatel changed the title remove mavis, change on-dock-removed to event dock-removed remove mavis, change on-dock-removed to event dock.removed Dec 6, 2015
@anandkumarpatel
Copy link
Copy Markdown
Contributor Author

tested on beta, works like a 🍀

@anandkumarpatel
Copy link
Copy Markdown
Contributor Author

steps for deployment

Comment thread test/functional/fixtures/dock.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it's a bit chaotic to use count.inc() and createCount(#, done) in a single test. (Specifying a number of callbacks at initialization and then incrementing it)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@cflynn07
Copy link
Copy Markdown
Contributor

cflynn07 commented Dec 6, 2015

Changes LGTM. I also reviewed associated PR.

@podviaznikov
Copy link
Copy Markdown
Member

code looks good

@podviaznikov
Copy link
Copy Markdown
Member

works on beta!

anandkumarpatel pushed a commit that referenced this pull request Dec 7, 2015
remove mavis, change on-dock-removed to event dock.removed
@anandkumarpatel anandkumarpatel merged commit c84054d into master Dec 7, 2015
@anandkumarpatel anandkumarpatel deleted the SAN-3042-remove-docks-from-consul branch December 7, 2015 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants